Skip to content

Wizard: prevent reloading of Wizard on enter#4258

Merged
regexowl merged 1 commit intoosbuild:mainfrom
avitova:fix-reaload-on-enter
Apr 2, 2026
Merged

Wizard: prevent reloading of Wizard on enter#4258
regexowl merged 1 commit intoosbuild:mainfrom
avitova:fix-reaload-on-enter

Conversation

@avitova
Copy link
Copy Markdown
Collaborator

@avitova avitova commented Mar 31, 2026

I noticed this bug during the UX call.

In LabelInput, the Wizard requires users to press enter. Although the hostname step does not need it, users might press enter without knowing it is not needed, which previously trigerred a re-render. This commit fixes the re-render.

@avitova avitova requested a review from a team as a code owner March 31, 2026 14:34
@avitova avitova changed the title Wizard: prevent to reload Wizard on enter Wizard: prevent reloading of Wizard on enter Mar 31, 2026
Copy link
Copy Markdown

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • Consider extracting the inline onSubmit handler into a named function (e.g., handleSubmit) to keep the JSX cleaner and make the intent of preventing default submission easier to reuse or adjust if the step’s behavior changes later.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider extracting the inline onSubmit handler into a named function (e.g., handleSubmit) to keep the JSX cleaner and make the intent of preventing default submission easier to reuse or adjust if the step’s behavior changes later.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 31, 2026

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.52%. Comparing base (8b1ff13) to head (e2eed47).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...ponents/CreateImageWizard/steps/Hostname/index.tsx 0.00% 2 Missing ⚠️

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4258      +/-   ##
==========================================
- Coverage   71.54%   71.52%   -0.02%     
==========================================
  Files         199      199              
  Lines        7524     7526       +2     
  Branches     2813     2813              
==========================================
  Hits         5383     5383              
- Misses       1867     1869       +2     
  Partials      274      274              
Files with missing lines Coverage Δ
...ponents/CreateImageWizard/steps/Hostname/index.tsx 50.00% <0.00%> (-50.00%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8b1ff13...e2eed47. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@regexowl
Copy link
Copy Markdown
Collaborator

regexowl commented Apr 1, 2026

Oh, this is great catch! This might be an issue in general, not just for the hostname input, right?

@avitova
Copy link
Copy Markdown
Collaborator Author

avitova commented Apr 1, 2026

@regexowl Yes, and we should maybe try to catch this even in our playwright tests, what do you think? I did not see this happening anywhere else, but the "Form" component behaves like this normally.

In LabelInput, the Wizard requires users to press enter. Although the
hostname step does not need it, users might press enter without knowing
it is not needed, which previously trigerred a re-render. This commit
fixes the re-render.
@avitova avitova force-pushed the fix-reaload-on-enter branch from ebbfb8c to e2eed47 Compare April 1, 2026 13:03
@regexowl
Copy link
Copy Markdown
Collaborator

regexowl commented Apr 1, 2026

@regexowl Yes, and we should maybe try to catch this even in our playwright tests, what do you think?

Agree, we should

I did not see this happening anywhere else, but the "Form" component behaves like this normally.

Just tried and the repositories search input behaves the same way. What solves it is adding a handler for the submit and then passing that to the Forms, BUT... since we're somewhat in the middle of the revamp we have some nested forms that just throw the entire thing out of the window 😓

Managed to resolve it for the Repositories and packages step: #4263 (had to gate the forms first and then add the handler)

We can either start gating the other steps in the same way, or we can put a big pin in this and revisit once the steps are rendered correctly without the nesting. What do you think?

@avitova
Copy link
Copy Markdown
Collaborator Author

avitova commented Apr 1, 2026

@regexowl I am okay with revisiting after the revamp is done. Well, we could use the preventDefault for the fields where we noticed this behaviour, and create a ticket in order to not forget fixing this later in a more elegant way + adding tests. What do you think?

@regexowl
Copy link
Copy Markdown
Collaborator

regexowl commented Apr 2, 2026

@regexowl I am okay with revisiting after the revamp is done. Well, we could use the preventDefault for the fields where we noticed this behaviour, and create a ticket in order to not forget fixing this later in a more elegant way + adding tests. What do you think?

Sure! We can always revise the problematic inputs later. Let's get this in, thank you! 🚀

@regexowl regexowl added this pull request to the merge queue Apr 2, 2026
@regexowl
Copy link
Copy Markdown
Collaborator

regexowl commented Apr 2, 2026

Opened a task for the tests: https://redhat.atlassian.net/browse/HMS-10462 we can revise the functionality within it

Merged via the queue into osbuild:main with commit a40080d Apr 2, 2026
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants